🌱 v1beta2: implement VSphereCluster's Ready, FailureDomainsReady, ClusterModulesReady and VCenterAvailable conditions for govmomi #3396
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/XXL
|
/hold For rebase after merged |
k8s-ci-robot
added
the
do-not-merge/hold
chrischdi
force-pushed
the
pr-govmomi-vspherecluster
branch
3 times, most recently
from
c052885 to
ed5924a
Compare
k8s-ci-robot
added
the
needs-rebase
chrischdi
force-pushed
the
pr-govmomi-vspherecluster
branch
from
ed5924a to
dd8d3d6
Compare
k8s-ci-robot
removed
the
needs-rebase
chrischdi
changed the title
k8s-ci-robot
added
size/L
chrischdi
force-pushed
the
pr-govmomi-vspherecluster
branch
from
dd8d3d6 to
70e9fbb
Compare
chrischdi
force-pushed
the
pr-govmomi-vspherecluster
branch
from
70e9fbb to
d219994
Compare
| ), | ||
| ) | ||
|
|
||
| if !v1beta2conditions.Has(clusterCtx.VSphereCluster, infrav1.VSphereClusterVCenterAvailableV1Beta2Condition) { |
There was a problem hiding this comment.
I'm wondering a little bit about this.
If the goal is to handle code paths where those conditions are not set after an upgrade, this could lead to a state where the condition is false and it is never moved to true, which is an issue.
What if instead, if the condition is missing, we set this condition to true if the vsphere cluster is ready, false otherwise
There was a problem hiding this comment.
ack, misinterpreted what was discussed :-)
Adjusting it to set them on ignore. The old summary only included VCenterAvailable, so we also will rely on that, and add the others with ignore.
chrischdi
added
the
tide/merge-method-squash
|
/retest Image pull failure while building kind image |
…eady and VCenterAvailable v1beta2 conditions for govmomi
chrischdi
force-pushed
the
pr-govmomi-vspherecluster
branch
from
8333790 to
a88b513
Compare
|
@chrischdi: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Thx! /lgtm /assign @fabriziopandini |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: a4608be7aa736b22d9d981c501cc16451677f4fe
|
| // VSphereCluster's VCenterAvailable condition and corresponding reasons that will be used in v1Beta2 API version. | ||
| const ( | ||
| // VSphereClusterVCenterAvailableV1Beta2Condition documents the status of vCenter for a VSphereCluster. | ||
| VSphereClusterVCenterAvailableV1Beta2Condition = "VCenterAvailable" | ||
|
|
||
| // VSphereClusterVCenterAvailableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is available. | ||
| VSphereClusterVCenterAvailableV1Beta2Reason = clusterv1.AvailableV1Beta2Reason | ||
|
|
||
| // VSphereClusterVCenterUnreachableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is unreachable. | ||
| VSphereClusterVCenterUnreachableV1Beta2Reason = "VCenterUnreachable" | ||
|
|
||
| // VSphereClusterVCenterNotAvailableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is not available. | ||
| VSphereClusterVCenterNotAvailableV1Beta2Reason = clusterv1.NotAvailableV1Beta2Reason | ||
|
|
||
| // VSphereClusterVCenterAvailableDeletingV1Beta2Reason surfaces when the VSphereCluster is being deleted. | ||
| VSphereClusterVCenterAvailableDeletingV1Beta2Reason = clusterv1.DeletingV1Beta2Reason | ||
| ) |
There was a problem hiding this comment.
We should look into making this condition consistent across objects
| case len(modErrs) == 0 && len(clusterModuleSpecs) > 0: | ||
| conditions.MarkTrue(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition) | ||
| default: | ||
| conditions.Delete(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition) | ||
| } | ||
| v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ |
There was a problem hiding this comment.
q: why this is not after L188
What this PR does / why we need it:
Implements the following v1beta2 conditions for govmomi mode's VSphereCluster
Part of #3221
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #